Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: migrate from travis to github action #391

Merged
merged 9 commits into from
Dec 15, 2021
Merged

ci: migrate from travis to github action #391

merged 9 commits into from
Dec 15, 2021

Conversation

chenrui333
Copy link
Contributor

@chenrui333 chenrui333 commented Dec 10, 2021


closes #378

Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Copy link
Member

@mirogta mirogta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@natefinch - this drops go 1.9 and 1.10 from the tests. I know how strongly you feel about mage's backwards combability - are you also happy with removing those?

@natefinch
Copy link
Member

Is there any reason we can't support 1.9 and 1.10 as well?

@chenrui333
Copy link
Contributor Author

chenrui333 commented Dec 13, 2021

Is there any reason we can't support 1.9 and 1.10 as well?

@natefinch Go modules was introduced in go1.11, that is the main reason why it does not work for them.

@natefinch
Copy link
Member

Oh yeah, lol, yeah, that seems fine.

Copy link
Member

@natefinch natefinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaney071
Copy link

thaney071 commented Dec 14, 2021

They should still be backwards compatible. The go tools changed behaviors at go 1.12 to be module aware.

The go directive in a go.mod file now indicates the version of the language used by the files within that module. It will be set to the current release (go 1.12) if no existing version is present. If the go directive for a module specifies a version newer than the toolchain in use, the go command will attempt to build the packages regardless, and will note the mismatch only if that build fails.

This changed use of the go directive means that if you use Go 1.12 to build a module, thus recording go 1.12 in the go.mod file, you will get an error when attempting to build the same module with Go 1.11 through Go 1.11.3. Go 1.11.4 or later will work fine, as will releases older than Go 1.11. If you must use Go 1.11 through 1.11.3, you can avoid the problem by setting the language version to 1.11, using the Go 1.12 go tool, via go mod edit -go=1.11.

The CI for 1.11.x probably passes because it is defaulting to 1.11.4 which is supported by the repos go.mod version directive.

For builds in 1.10.x and 1.9.x, they need to be setup to be in the GOPATH for them to work. I see that you set the GOPATH variable for testing, but the code itself needs to checked out to the GOPATH for the to tooling to work, and it must be in the correct layout for the imports to work.

I tried debugging a bit more, but am not as familiar with github actions. As far as I can tell the checkout is not following the expected layout for go version 1.10 and 1.9 to work. see: https://github.com/thaney071/mage/actions

Sorry, long post to say the builds should be able to still work using 1.10 and 1.9, but the checkout step needs to be configured so the code is checked out to the GOPATH as: $GOPATH/src/github.com/mage/mage

@natefinch seems I took to long to post this.

@natefinch
Copy link
Member

This isn't getting picked up in the "checks" section.... I always forget if I need to do something special to turn these things on, or is it just that this is an outside contributor, so it doesn't actually run on the PR?

@chenrui333
Copy link
Contributor Author

Sorry, long post to say the builds should be able to still work using 1.10 and 1.9, but the checkout step needs to be configured so the code is checked out to the GOPATH as: $GOPATH/src/github.com/mage/mage

Yeah, if we still want to make the builds successful with 1.9 or 1.10, I agree that we can configure GOPATH.

But soon or later, we will need to form a plan to remove go1.9 and go1.10 support.

@chenrui333
Copy link
Contributor Author

This isn't getting picked up in the "checks" section.... I always forget if I need to do something special to turn these things on, or is it just that this is an outside contributor, so it doesn't actually run on the PR?

Since github action is not on the base branch yet, it did not get run. After the merge it would run on the PRs. (that is why I posted the action runs on my fork)

@mirogta
Copy link
Member

mirogta commented Dec 15, 2021

But soon or later, we will need to form a plan to remove go1.9 and go1.10 support.

@natefinch In relation to this, have you thought about adding some anonymous telemetry collection when mage is executed, which could be of course turned off, but perhaps on by default?

OS version, go version, build duration, which features of mage are used? How many are using mage with go 1.9/1.10, or mage flags (-w etc.), or mage functions (sh.Run etc.), and more… To get a better picture than what we get just from people telling us here or in a chat. Something like https://yarnpkg.com/advanced/telemetry

I've added ^^^ for the discussion here not to pollute this PR.

@thaney071
Copy link

If @natefinch and @mirogta are okay with not testing on go 1.9 and 1.10, this can be merged. I can keep looking at the action setting or update the docs/web template for the tested versions.

The website has 1.7 as the lowest tested as of today.

@natefinch
Copy link
Member

I think testing on 1.11+ should be fine. That's fall 2018 and was when modules were first added as opt-in.

@natefinch natefinch merged commit 2f1ec40 into magefile:master Dec 15, 2021
@chenrui333 chenrui333 deleted the add-github-action-ci branch December 15, 2021 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CI to use stable 1.16 go
4 participants